Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
chrismaz11
left a comment
There was a problem hiding this comment.
PR Review — Signing Key Versioning & Persistent Replay Protection
Summary
This PR strengthens TrustSignal’s security architecture by implementing two previously documented controls:
- Receipt signing key versioning
- Persistent GitHub webhook replay protection
Both changes convert assumptions in the Threat Model into runtime-enforced behavior, with regression tests covering the critical security boundaries.
These changes materially improve receipt integrity guarantees and webhook ingestion safety while maintaining backward compatibility.
- Receipt Signing Key Versioning
What was implemented
Newly issued receipts now include signing_key_id.
Verification logic:
- resolves the correct verification key using
signing_key_id - fails closed if the referenced key is unknown or mismatched
- preserves compatibility for existing receipts without the field
Older receipts remain verifiable via the existing kid-based lookup.
kid remains the canonical JWK identifier, while signing_key_id provides deterministic receipt-level key selection and lifecycle management.
Why this matters
Without key versioning:
- rotating signing keys can invalidate historical receipts
- multi-key deployments introduce verification ambiguity
- compromised key containment becomes difficult
With signing_key_id:
- receipts remain verifiable across key rotations
- multi-key environments behave deterministically
- compromised keys can be revoked without breaking receipt history
Security invariant
signing_key_id is cryptographically bound to the signed payload.
It is included within the canonicalized receipt payload that is signed and verified, preventing key-ID substitution without invalidating the signature.
- Signing Configuration Compatibility
What was implemented
Added support for environment variable aliases:
TRUSTSIGNAL_SIGNING_KEY_ID
TRUSTSIGNAL_SIGNING_PRIVATE_JWK
TRUSTSIGNAL_PUBLIC_JWKS
Existing configuration via TRUSTSIGNAL_RECEIPT_SIGNING_* continues to work for backward compatibility.
Recommendation
Define a single canonical environment variable scheme in documentation and treat the others as compatibility aliases during rollout.
Secrets should continue to follow strict logging rules.
- Persistent Webhook Replay Protection
What was implemented
Replay protection for GitHub webhooks now uses a persistent SQLite-backed store.
Replay table structure:
delivery_id (primary key)
first_seen_at
Processing order:
- Verify webhook signature
- Check replay store
- Reject duplicates with 409 Conflict
TTL pruning allows entries to expire.
Why this matters
Memory-only replay protection fails under:
- service restart
- container redeploy
- crash recovery
Persistent replay storage closes those gaps.
- Deployment Consideration
Replay protection currently relies on a local SQLite database.
Replay protection strength depends on deployment model:
Single instance with persistent disk — Strong
Container with durable volume — Strong
Horizontally scaled multi-instance — Partial
Serverless / ephemeral filesystem — Weak
This does not block merging the PR but should be documented to avoid overstating replay guarantees.
Future deployments in serverless or multi-instance environments should use a shared durable replay store.
- Tests
Receipt signing tests
- Receipts include signing_key_id
- Verification succeeds with correct key
- Verification fails with unknown key
- Old receipts without signing_key_id remain verifiable
Replay protection tests
- First delivery accepted
- Duplicate delivery rejected with 409
- Invalid signature rejected before replay logic
- Replay persistence verified across restart
Validation Results
trustsignal
- typecheck: passed
- test: passed
- build: passed
- lint: failed (pre-existing violations)
TrustSignal-App
- validate: passed
Lint failures appear unrelated to this PR.
Security Impact
Receipt signing key lifecycle — Implemented
Webhook replay protection persistence — Implemented
Receipt verification backward compatibility — Preserved
Signature verification-before-replay ordering — Preserved
Fail-closed verification behavior — Enforced
This PR converts previously documented security controls into runtime-enforced guarantees.
Recommendation
Looks good to merge with minor notes.
Follow-up actions:
- Document deployment assumptions for the SQLite replay store.
- Standardize canonical environment variable naming for signing configuration.
- Address existing lint violations in a separate cleanup PR.
Overall this is a correct, minimal, backward-compatible security hardening that materially improves TrustSignal’s integrity guarantees.
chrismaz11
left a comment
There was a problem hiding this comment.
PR Review — Signing Key Versioning & Persistent Replay Protection
Summary
This PR strengthens TrustSignal’s security architecture by implementing two previously documented controls:
- Receipt signing key versioning
- Persistent GitHub webhook replay protection
Both changes convert assumptions in the Threat Model into runtime-enforced behavior, with regression tests covering the critical security boundaries.
These changes materially improve receipt integrity guarantees and webhook ingestion safety while maintaining backward compatibility.
- Receipt Signing Key Versioning
What was implemented
Newly issued receipts now include signing_key_id.
Verification logic:
- resolves the correct verification key using
signing_key_id - fails closed if the referenced key is unknown or mismatched
- preserves compatibility for existing receipts without the field
Older receipts remain verifiable via the existing kid-based lookup.
kid remains the canonical JWK identifier, while signing_key_id provides deterministic receipt-level key selection and lifecycle management.
Why this matters
Without key versioning:
- rotating signing keys can invalidate historical receipts
- multi-key deployments introduce verification ambiguity
- compromised key containment becomes difficult
With signing_key_id:
- receipts remain verifiable across key rotations
- multi-key environments behave deterministically
- compromised keys can be revoked without breaking receipt history
Security invariant
signing_key_id is cryptographically bound to the signed payload.
It is included within the canonicalized receipt payload that is signed and verified, preventing key-ID substitution without invalidating the signature.
- Signing Configuration Compatibility
What was implemented
Added support for environment variable aliases:
TRUSTSIGNAL_SIGNING_KEY_ID
TRUSTSIGNAL_SIGNING_PRIVATE_JWK
TRUSTSIGNAL_PUBLIC_JWKS
Existing configuration via TRUSTSIGNAL_RECEIPT_SIGNING_* continues to work for backward compatibility.
Recommendation
Define a single canonical environment variable scheme in documentation and treat the others as compatibility aliases during rollout.
Secrets should continue to follow strict logging rules.
- Persistent Webhook Replay Protection
What was implemented
Replay protection for GitHub webhooks now uses a persistent SQLite-backed store.
Replay table structure:
delivery_id (primary key)
first_seen_at
Processing order:
- Verify webhook signature
- Check replay store
- Reject duplicates with 409 Conflict
TTL pruning allows entries to expire.
Why this matters
Memory-only replay protection fails under:
- service restart
- container redeploy
- crash recovery
Persistent replay storage closes those gaps.
- Deployment Consideration
Replay protection currently relies on a local SQLite database.
Replay protection strength depends on deployment model:
Single instance with persistent disk — Strong
Container with durable volume — Strong
Horizontally scaled multi-instance — Partial
Serverless / ephemeral filesystem — Weak
This does not block merging the PR but should be documented to avoid overstating replay guarantees.
Future deployments in serverless or multi-instance environments should use a shared durable replay store.
- Tests
Receipt signing tests
- Receipts include signing_key_id
- Verification succeeds with correct key
- Verification fails with unknown key
- Old receipts without signing_key_id remain verifiable
Replay protection tests
- First delivery accepted
- Duplicate delivery rejected with 409
- Invalid signature rejected before replay logic
- Replay persistence verified across restart
Validation Results
trustsignal
- typecheck: passed
- test: passed
- build: passed
- lint: failed (pre-existing violations)
TrustSignal-App
- validate: passed
Lint failures appear unrelated to this PR.
Security Impact
Receipt signing key lifecycle — Implemented
Webhook replay protection persistence — Implemented
Receipt verification backward compatibility — Preserved
Signature verification-before-replay ordering — Preserved
Fail-closed verification behavior — Enforced
This PR converts previously documented security controls into runtime-enforced guarantees.
Recommendation
Looks good to merge with minor notes.
Follow-up actions:
- Document deployment assumptions for the SQLite replay store.
- Standardize canonical environment variable naming for signing configuration.
- Address existing lint violations in a separate cleanup PR.
Overall this is a correct, minimal, backward-compatible security hardening that materially improves TrustSignal’s integrity guarantees.
Summary
AI Disclosure
Review Checklist